Skip to content

CloudFederationApi: access-token lifecycle fixes#61889

Draft
mickenordin wants to merge 3 commits into
masterfrom
kano-fix-ocm-duplicate-sharedSecret
Draft

CloudFederationApi: access-token lifecycle fixes#61889
mickenordin wants to merge 3 commits into
masterfrom
kano-fix-ocm-duplicate-sharedSecret

Conversation

@mickenordin

@mickenordin mickenordin commented Jul 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Fixes three lifecycle bugs in the OCM access-token exchange

  1. A refresh token (the OCM sharedSecret) can back several concurrent access tokens.

  2. CleanupExpiredOcmTokensJob only deleted ocm_token_map not the access tokens in oc_authtoken

  3. A new ShareDeletedEvent listener revokes all access tokens exchanged from the removed share's secret

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Tests (unit, integration, api and/or acceptance) are included
  • Screenshots N/A because only a backend change
  • Documentation is not required
  • Backports not neccessary
  • Labels added
  • Milestone added

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

@mickenordin mickenordin added this to the Nextcloud 35 milestone Jul 7, 2026
@mickenordin mickenordin requested a review from enriquepablo July 7, 2026 17:35
@mickenordin mickenordin added bug 3. to review Waiting for reviews labels Jul 7, 2026
@mickenordin mickenordin force-pushed the kano-fix-ocm-duplicate-sharedSecret branch from 9c60ec5 to 83513fa Compare July 7, 2026 17:37
@mickenordin
fix(cloud_federation_api): revoke OCM access tokens on expiry cleanup
4f7ab0d 
The cleanup job only deleted ocm_token_map rows, orphaning the
temporary access tokens in oc_authtoken so they accumulated
indefinitely. Revoke each expired access token before dropping its
mapping.

Assisted-by: ClaudeCode:claude-fable-5

Signed-off-by: Micke Nordin <kano@sunet.se>
@mickenordin
fix(cloud_federation_api): revoke OCM tokens when a federated share i…
62fcb22 
…s removed

Extract the revoke-then-delete logic into OcmTokenService and add a
ShareDeletedEvent listener that revokes the access tokens exchanged from
a removed share's secret, drops their ocm_token_map rows, and invalidates
the refresh token — instead of leaving them until the expiry job, which
can no longer find them once the share is gone.

Assisted-by: ClaudeCode:claude-fable-5

Signed-off-by: Micke Nordin <kano@sunet.se>
@mickenordin
fix(cloud_federation_api): allow multiple access tokens per refresh t…
60f962d 
…oken

A refresh token may back several concurrent access tokens. E.g. in a multi
protocol share for a webapp the webdav mount and the webapp launcher exchange
the same secret, so the exchange must not assume a single mapping.

Drop the revoke-previous step and the findByRefreshToken (findEntity) lookup, which threw
MultipleObjectsReturnedException once more than one access token existed.
Expiry and unshare cleanup revoke the tokens instead.

Assisted-by: ClaudeCode:claude-fable-5

Signed-off-by: Micke Nordin <kano@sunet.se>
@mickenordin mickenordin force-pushed the kano-fix-ocm-duplicate-sharedSecret branch from 83513fa to 60f962d Compare July 8, 2026 06:45
@mickenordin

mickenordin commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

I was able to verify this in my testrig:

kubectl --context ocm-test -n nextcloud exec alice-db-1 -- \
  psql -U postgres -d alice -c \
  "SELECT count(*) AS maps_for_secret
     FROM oc_ocm_token_map
    WHERE refresh_token = '<verysecret>';
   SELECT count(*) AS total_ocm_access_tokens
     FROM oc_authtoken WHERE name = 'OCM Access Token';"
Defaulted container "postgres" out of: postgres, bootstrap-controller (init)
 maps_for_secret 
-----------------
               3
(1 row)

 total_ocm_access_tokens 
-------------------------
                       3
(1 row)

Then after unshare:

kubectl --context ocm-test -n nextcloud exec alice-db-1 -- \
  psql -U postgres -d alice -c \
  "SELECT count(*) AS maps_for_secret
     FROM oc_ocm_token_map
    WHERE refresh_token = '<verysecret>';
   SELECT count(*) AS total_ocm_access_tokens
     FROM oc_authtoken WHERE name = 'OCM Access Token';"
Defaulted container "postgres" out of: postgres, bootstrap-controller (init)
 maps_for_secret 
-----------------
               0
(1 row)

 total_ocm_access_tokens 
-------------------------
                       0
(1 row)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@enriquepablo enriquepablo Awaiting requested review from enriquepablo

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

3. to review Waiting for reviews AI assisted bug

Projects

None yet

Milestone

Nextcloud 35

Development

Successfully merging this pull request may close these issues.

2 participants

@mickenordin @nextcloud-command